Skip to content

Conversation

darkwisebear
Copy link

Add the possibility to use shared trivial types as deleters for objects managed by std::unique_ptr / UniquePtr.

Add the possibility to use shared trivial types as deleters for objects managed
by std::unique_ptr / UniquePtr.
GCC does not accept an extern "C" storage class specifier along with
definition of the global variable. Split declaration and definition to
overcome this.
Apparently MacOS's unique_ptr implementation puts the
deleter as the 2nd member. Reflect this in the unique_ptr definition.
* Add unnamed lifetimes to drop implementation for UniquePtrTarget
* Update expected stderr to include the deleter type parameter.
@darkwisebear
Copy link
Author

@dtolnay Does this PR have chances to be merged as-is or is there still something I need to do with it in order to get it merged?

@darkwisebear
Copy link
Author

Well, now this guy has a merge conflict. It shouldn't be a big deal to fix it but is it really worth it? Was it laying around for such a long time because the change is bad? If yes, please tell me.

On a more general side, is it worth that I also add allocator support to vector and shared_ptr? Probably not because I feel that for some reason there is no chance of such changes to ever getting merged. If that is due to lack of time I'd be glad to help here. There's a lot of pending PRs that would be embraced by the community and maybe a lot of stuff that people would be willing to contribute I guess.

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Comment on lines +24 to +38
#[cfg(not(target_os = "macos"))]
#[repr(C)]
struct UniquePtrInner<T, D> {
deleter: D,
repr: MaybeUninit<*mut c_void>,
ty: PhantomData<T>,
}

impl<T> UniquePtr<T>
#[cfg(target_os = "macos")]
#[repr(C)]
struct UniquePtrInner<T, D> {
repr: MaybeUninit<*mut c_void>,
deleter: D,
ty: PhantomData<T>,
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not something I am willing to assume about the internal layout of std::unique_ptr.

Can you suggest some other way to implement this feature?

@dtolnay
Copy link
Owner

dtolnay commented Aug 10, 2025

I am pretty confident that this general approach is not a good fit for this library. Depending on exactly how you were planning to use this, I can recommend some alternatives:

  1. Use a shared ptr that just always has one reference and is moved like a unique ptr. Shared pointers have a type-erased deleter so the Rust SharedPtr binding already works with custom deleters.

  2. Use using UniqueC = std::unique_ptr<C, PmrDeleterForC> in C++ and write your own Rust struct UniqueC and unsafe impl ExternType for UniqueC, making whatever layout assumptions you consider appropriate for the set of standard libraries and platforms that your software is targeting.

  3. Use using UniqueC = std::unique_ptr<C, PmrDeleterForC> and wire up Bindgen to produce the Rust struct for it with the correct layout for the user's platform, adding a manual ExternType impl on the Bindgen-generated struct to make it usable in the bridge module by value.

  4. Use using UniqueC = std::unique_ptr<C, PmrDeleterForC> and then use UniqueC as an opaque C++ type in the bridge module, i.e. double boxing.

  5. Make your own type-erased unique pointer struct with a well-defined layout. Wrap it in a Rust binding that attaches type information using PhantomData.

@dtolnay dtolnay closed this Aug 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants